[dart-dio][built_value] Register BuilderFactory for nested additionalProperties shapes#23662
Open
Homegan wants to merge 2 commits intoOpenAPITools:masterfrom
Open
Conversation
For a property like
`{ additionalProperties: { type: array, items: { $ref: ... } } }` the
generator emits `Map<String, BuiltList<X>>` in the Dart class but never
registers the `BuiltList<X>` BuilderFactory. The only
factory-registration path that ran for additionalProperties looked at
`items.getAdditionalProperties()`, which is null for this very common
shape (a region map of arrays of $ref). built_value then fails at
runtime with `Bad state: No builder factory for BuiltList<X>` on the
first deserialization that touches the property.
Fix:
- `postProcessModelProperty` now also calls `registerNestedBuilderFactories`,
which walks the property's `items` tree top-down and registers a
factory for every container layer. Three small helpers
(`renderInnerFullType`, `renderDartType`, `renderBuilderFactory`)
compute the corresponding `FullType(...)` argument list and the
matching `XBuilder<...>` instantiation for arbitrary nesting:
`Map<String, List<X>>`, `List<Map<String, X>>`,
`Map<String, Map<String, X>>`, `List<List<X>>`, `Set<...>`, etc.
- `BuiltValueSerializer` gets a new `composite(fullTypeArgs,
builderInstantiation)` constructor that carries the pre-rendered
expressions. The existing `(isArray, uniqueItems, isMap,
isNullable, dataType)` form is unchanged -- needed because the
original model can't represent something like
`BuiltMap<String, BuiltList<X>>` (the `FullType` argument is
recursive and isn't expressible with a single `dataType` string).
`equals`/`hashCode` are extended so composite serializers dedup on
`(fullTypeArgs, builderInstantiation)` and never collide with simple
ones.
- `serializers.mustache` gets a new branch that emits the composite
fields verbatim when present; otherwise the existing
`isArray`/`isMap` dispatch runs unchanged.
Existing simple cases (direct return / parameter container types,
single-level additionalProperties already handled by the prior
branch) keep producing byte-identical output.
A new fixture `built_value_additional_properties_factory.yaml`
exercises the canonical `Map<String, List<$ref>>` shape, and a new
test
`DartDioClientCodegenTest.testNestedAdditionalPropertiesGetBuilderFactories`
asserts both the inner `BuiltList<WatchProviderEntry>` and the outer
`BuiltMap<String, BuiltList<WatchProviderEntry>>` factories appear in
the generated `serializers.dart`.
Full Dart suite: 115 tests, 0 failures, 0 regressions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[dart-dio][built_value]RegisterBuilderFactoryfor nestedadditionalPropertiesshapesPR checklist
[dart-dio][built_value].master.Summary
For a property like
{ additionalProperties: { type: array, items: { $ref: ... } } }(a "map of array of$ref" — a very common shape: category → list of items, locale → list of strings, tag → list of IDs), the dart-dio built_value generator emits a Dart property of typeBuiltMap<String, BuiltList<X>>but never registers theBuiltList<X>BuilderFactory inserializers.dart. built_value then fails at runtime with:The code compiles fine — the failure only surfaces on the first deserialization that touches the property.
Reproduction
Generated
serializers.dartbefore this PR:No factory for
BuiltList<Widget>.Root cause
DartDioClientCodegen.postProcessModelPropertyalready had a branch foradditionalProperties, but it only fired whenitems.getAdditionalProperties()itself was non-null — i.e. nestedMap<String, Map<String, X>>. The much more common shapeMap<String, List<X>>(a category map of arrays of$ref) was missed entirely:Worse, even if the right tree was being walked, the existing
BuiltValueSerializermodel only carries a singledataTypestring — physically can't represent something likeBuiltMap<String, BuiltList<X>>, because theFullTypeargument is recursive (FullType(BuiltMap, [FullType(String), FullType(BuiltList, [FullType(X)])])) and isn't expressible with(isArray | isMap, dataType).Fix
Two pieces:
1. Recursive walk in Java
postProcessModelPropertynow also callsregisterNestedBuilderFactories(property), which walks the property'sitemstree top-down and registers a factory for every container layer. Three small helpers compute the corresponding strings for arbitrary nesting:Map<String, List<X>>BuiltList<X>,BuiltMap<String, BuiltList<X>>List<Map<String, X>>BuiltMap<String, X>,BuiltList<BuiltMap<String, X>>Map<String, Map<String, X>>BuiltMap<String, X>,BuiltMap<String, BuiltMap<String, X>>List<List<X>>BuiltList<X>,BuiltList<BuiltList<X>>Set<...>variantsBuiltList/ListBuilderforBuiltSet/SetBuilderRecursion handles deeper trees naturally.
2. Composite mode for
BuiltValueSerializerA new constructor
BuiltValueSerializer.composite(fullTypeArgs, builderInstantiation)carries the pre-renderedFullType(...)arguments andXBuilder<...>instantiation.serializers.mustachegets a new branch that emits them verbatim:equals/hashCodeare extended so composite serializers dedup on(fullTypeArgs, builderInstantiation)and never collide with simple ones.After this PR:
The existing single-level cases continue to flow through the original
(isArray | isMap, dataType)dispatch — no behavior change for them, just fewer "No builder factory" errors at the edges.Tests
A new fixture
src/test/resources/3_0/dart-dio/built_value_additional_properties_factory.yamlexercises the canonicalMap<String, List<$ref>>shape on a minimalWidget/WidgetsByCategory/Catalogschema. A new testDartDioClientCodegenTest.testNestedAdditionalPropertiesGetBuilderFactoriesasserts both the innerBuiltList<Widget>and the outerBuiltMap<String, BuiltList<Widget>>factories appear inserializers.dart.Files changed
4 files, +252 / 0.
Compatibility
BuiltValueSerializerpublic surface: only additive (a new constructor, two new fields). The existing constructor and fields are unchanged. No change to the simpleisArray | isMaptemplate dispatch...addBuilderFactory(...)lines only appear when a property's container tree is nested (rare today because of this very bug — most users avoid the shape).Related
#23645 (
[BUG][DART-DIO] Invalid generics for additionalProperties without type) is a different, adjacent bug in the same area — it's about theBuiltMap<String>(single type parameter) compile error when the schema hasadditionalPropertiesbut notype: object. That one is at the schema-classification layer (setTypeProperties/getPrimitiveType); this PR is at the factory-registration layer inDartDioClientCodegen. Both can land independently. Worth keeping an eye on while reviewing.Summary by cubic
Fixes missing and duplicate
BuilderFactoryregistrations for nestedadditionalPropertiesin thedart-diobuilt_valuegenerator. Ensuresserializers.dartincludes the correct factories for shapes likeMap<String, List<Widget>>, preventing “Bad state: No builder factory for BuiltList” at runtime.BuiltList<Widget>andBuiltMap<String, BuiltList<Widget>>).BuiltValueSerializerto emit nestedFullType(...)/XBuilderpairs; updatesserializers.mustacheand deduplicates simple/composite entries to avoid duplicateaddBuilderFactorylines.petstoresample to reflect new factories.Written for commit 70521cd. Summary will update on new commits. Review in cubic